Support buffer protocol objects as advanced index keys#2889
Support buffer protocol objects as advanced index keys#2889vlad-perevezentsev wants to merge 3 commits intofix_inplace_indexind_4dfrom
Conversation
|
Array API standard conformance tests for dpnp=0.21.0dev0=py313h509198e_19 ran successfully. |
| if isinstance(x, numpy.ndarray): | ||
| return x | ||
| # convert buffer protocol objects (array.array, memoryview, etc.) | ||
| try: |
There was a problem hiding this comment.
seems __buffer__ has been added with 3.12: https://docs.python.org/3/reference/datamodel.html#object.__buffer__
https://docs.python.org/3/library/collections.abc.html#collections.abc.Buffer
https://peps.python.org/pep-0688/
we should probably start to support it as well. Maybe add a TODO to check for __buffer__/use collections.abc.Buffer and then call memoryview if it fails
There was a problem hiding this comment.
actually I misunderstood the PEP: this feature is already implemented, we can add a check for Python version and check against __buffer__ or try memoryview only if it's <3.12
| return arr | ||
| if isinstance(x, numpy.ndarray): | ||
| return x | ||
| # convert buffer protocol objects (array.array, memoryview, etc.) |
There was a problem hiding this comment.
Can all that use cases covered converting x to numpy.ndarray?
Do we need so complicated if-logic?
There was a problem hiding this comment.
here is CuPy implementation of a similar idea for a reference:
https://github.com/cupy/cupy/blob/main/cupy/_core/_routines_indexing.pyx#L247
can take a similar approach too in some cases (best to leave converting to usm_ndarray to the tensor layer, though, for queue propagation reasons)
There was a problem hiding this comment.
I don't think we need to fully follow the same CuPy implementation in _prepare_slice_list, because it seems CuPy mimics faulty NumPy behavior sometimes.
Not sure if I got the comment fully correctly, but in general I meant something like:
if x is None or x is Ellipsis or isinstance(x, (dpt.usm_ndarray, slice, numpy.ndarray)):
return x
if isinstance(x, dpnp_array):
return x.get_array()
try:
x = numpy.asarray(x)
except ... :
...
raise ...
...
return xThere was a problem hiding this comment.
yep, agreed mostly, though problem is, we don't really want to send integers or boolean Python scalars to numpy arrays, which is why we may need something like:
elif numpy.isscalar(s):
if not isinstance(s, (bool, numpy.bool_)):
# keep scalar int
continue
which is in cupy implementation. We could handle range, list, tuple through numpy though
This PR proposes by adding support for buffer protocol objects (
array.array,memoryviewetc.) as advanced index keysThese changes were proposed in #2872 as an extension to the advanced indexing support.